ℹ️ 🆒 Nice job cleaning up this 🌈 🗑️ .

I've never used moment before, so excuse me if this is a dumb question. does this mutate a global 'moment' reference?

Nope, not dumb, and you're right. I'm looking into the FormattedRelative component as a general replacement for this now, but I may either end up writing my own function or just sticking with moment as it doesn't look like FormattedRelative provides a simple way to retrieve elapsed time '3 seconds ago' => '3 seconds'. I can check with @lindseypoli & @marrasherrier, if they would be okay with that more relative phrasing, but that may be more confusing to the user.

is this function slow? do we run it on every frame, or are we memoizing?

Memoized in the processNodePositionsAndEdgeLineSegments selector which I believe only runs on the initial load of the process tree.

should this use i18n? different locales might wannt to reperenst this differently.

Did a search and found

Would that approach work here?

Yep, it should be using i18n. And thanks for that. I may be able to go with that, but it's gonna depend on how design feels about the subtle change in phrasing. It might actually be better for me to go with FormattedNumber or FormattedPlural for the way we have it designed at the moment

1. why is edgelinemetadata optional?
2. The edge line was represented as an array of vectors since it was just a geometric concept. Now that there is other data involved, how about a shape like this:

type EdgeLine = {
  points: [Vector2, Vector2];
  elapsedTime: number;
}

1. I'm guessing you meant optional? If so, it's because midway lines don't have any of that information

2. see below comment

If elaspedTime requires formatting, it may end up requiring DOM elements and the like. For that reason, perhaps it should be calculated in the react code.

If you went down that path, here's some ideas:
Instead of storing the (formatted) elasped time in state (or even storing the points) you could store the processes that the edge line connects:

type EdgeLine = {
  from: ResolverEvent;
  to: ResolverEvent;
}

Now the EdgeLine just represents the edge in the mathy-graphy sense.
Then you could add selectors like:

export function pointsForEdge(edgeLine: EdgeLine) {
  return [positionForNode(edgeLine.from), positionForNode(edgeLine.to) ]
}

export function relativeTimeForEdge(edgeLine: EdgeLine) {
  // im assuming that eventTimestamp is ms since epoch, if not, that conversion would be needed.
  return eventTimestamp(edgeLine.to) - eventTimestamp(edgeLine.from)
}

Then in the react code, you could get the points from poinntsForEdge by passing the edge (object reference itself.)

For rendering the time, you could do something like

<FormattedRelative value={relativeTimeForEdge(edgeLine)} />

That makes sense, and is definitely more extensible. I'll look into implementing it.

can you explain these magic numbers, and this expression.

I just modified the initial numbers set on 254 and 255 to better align on the graph. The function is just another magFactor dependent scale to help keep the position of the node centered as you scaled down and the event drop downs are removed from view.

can you explain this?

I can rename it and add a description. It sets the top % offset for the process buttons to keep them aligned as the resolver scales down

Not necessarily something you need to take on, but it might be good for use to try moving display logic into CSS where possible.

I see that this is older code. Any idea why we're using Object.entries here? We control the data structure (for the most part) so maybe we could change this.

Nope, but I can update it to just a single iteration. 👍

can you explain this expression?

Keeping the max-height of the button at 32, but it allows the sides of the button to better line up with the edge of the processNode

I think you should use useCallback here

I can throw it in one, but it may be counter productive since it really doesn't have any dependencies. There was this article I read a little while ago from Kent Dodds that goes into it a bit: https://kentcdodds.com/blog/usememo-and-usecallback

I'll add it to my reading list. Can you summarize here? I'll elaborate on my reasoning. Maybe it's wrong:

Any time this component renders, the closePopover reference will be new. The EuiPopover instance below is passed closePopover. If the EuiPopover component re-renders when its properties change, it will be guaranteed to rerender. By passing a memoized version of the closePopover function, we can avoid forcing EuiPopover to rerender.

If we force EuiPopover to rerender, then readers of this code will need to understand the performance complexity of EuiPopover in order to understand the perf complexity of this function (not meaning the strict technical definitions of those terms here.)

I read the article. This section talks about the use case I'm going for: https://kentcdodds.com/blog/usememo-and-usecallback#reactmemo-and-friends

More on closePopover:
In our particular case, EuiPopover fails to use React.memo or PureComponent. It will rerender on every frame. For that reason, I think you're correct: there is no performance benefit to using useCallback for closePopover. I think you're justified in leaving it out.

That being said, I would argue that EuiPopover should use PureComponent or useMemo. As it is, those components will rerender on every frame. Even the memory allocated for 'jsx' (e.g. just stuff like <div />) is substantial at that scale.

Also re: the article. While I agree w/ a lot of what is said there. The author suggests not to use React.memo. I would argue the opposite. In fact I'd argue that React shouldn't have any other way to create components.

Also re: the article. While I agree w/ a lot of what is said there. The author suggests not to use React.memo. I would argue the opposite. In fact I'd argue that React shouldn't have any other way to create components.

Completely agree with this

Would you mind adding a comment like

/**
 * Returns something. Used somehow.
 */

and would you mind adding an explicit return type.

Also, can you explain the reasoning for accepting either numbers or strings?

eventTimestamp depending on endgame event or current event will return either a string or number.

❔ just curious - why are we using different US offsets for the light mode gradients?

I thought the same thing, but just following the exported SVG I was provided 🤔

so in order to translate, you'd need to use a select here I guess? like:

"{durationType, select, day {días} weeks {semanas}}"

It was my thought that FormattedMessage actually translates this internally from what I see here https://formatjs.io/docs/react-intl/components/#formattedmessage and in other parts of Kibana